Skip to content

fix(sqlalchemy): use connection span in after cursor execute - #901

Merged
CagriYonca merged 1 commit into
mainfrom
fix-sqlalchemy
Sep 10, 2026
Merged

CagriYonca merged 1 commit into
mainfrom
fix-sqlalchemy

Conversation

@CagriYonca

Copy link
Copy Markdown
Contributor

What changed

receive_after_cursor_execute in sqlalchemy.py had two bugs:

  1. Guard check was dead code. get_tracer_tuple() returns a tuple, so if not tracer: was always False — the early-return path never ran.

  2. Wrong span was closed. get_current_span() reads from the global OTel context, which could be a different span than the one opened in before_cursor_execute. This corrupted the context stack and caused subsequent child spans (e.g. Redis) to be silently dropped.

Fix: close conn.span directly — the span stored on the connection object by before_cursor_execute — instead of reading from global context.

Tests

  • test_if_not_tracing: added assert len(spans) == 0
  • test_context_restored_after_query: new test asserting the parent span is active again after a SQL query completes

@CagriYonca
CagriYonca requested a review from a team as a code owner September 8, 2026 13:38
@CagriYonca CagriYonca self-assigned this Sep 8, 2026
@pvital pvital added the fix label Sep 8, 2026

@pvital pvital left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor request

Comment thread .gitignore Outdated
@CagriYonca
CagriYonca requested a review from pvital September 8, 2026 16:49
Signed-off-by: Cagri Yonca <cagri@ibm.com>

@pvital pvital left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good to me.

@CagriYonca
CagriYonca merged commit c2c3597 into main Sep 10, 2026
17 of 20 checks passed
@CagriYonca
CagriYonca deleted the fix-sqlalchemy branch September 10, 2026 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants